Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Domain only flow: redirect when starting the flow with a bad domain name #11823

Merged
merged 2 commits into from
Mar 8, 2017

Conversation

yurynix
Copy link
Contributor

@yurynix yurynix commented Mar 7, 2017

On domain only flow, when starting the flow with a bad domain name, redirect to /domains

Testing instructions

  1. Run git checkout add/error-message-for-no-domain and start your server, or open a live branch
  2. Open the domain only flow with a bad domain
  3. Assert that you are redirected
  4. Assert that with valid domain no error is displayed and there is no redirect

Reviews

  • Code
  • Product

@yurynix yurynix self-assigned this Mar 7, 2017
@matticbot
Copy link
Contributor

@matticbot matticbot added the [Size] M Medium sized issue label Mar 7, 2017
@yurynix yurynix added [Feature] Checkout The checkout screen and process for purchases made on WordPress.com. [Status] In Progress and removed [Size] M Medium sized issue labels Mar 7, 2017
@yurynix yurynix force-pushed the add/error-message-for-no-domain branch from 4513251 to 64f7b40 Compare March 7, 2017 10:40
@matticbot matticbot added the [Size] M Medium sized issue label Mar 7, 2017
@yurynix yurynix added [Status] Needs Copy Review Add this when you'd like to get a review / feedback from the Editorial team on your PR [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Mar 7, 2017
@ranh
Copy link
Contributor

ranh commented Mar 7, 2017

when starting the flow with a bad domain name, display an error message and redirect to the main flow

How about returning to the begining of the flow instead? Letting users choose a different domain, rather then defaulting them to the site flow.

I'm also not sure a visible notice is needed here. Unless we know how users ended up with a bad domain query in the first place, the notice is not going to be very useful.

@scruffian
Copy link
Member

The problem with keeping them in the domain-first flow is that there is no domain step in that flow, and adding one is problematic.

This solution looks fine to me. I propose we just go with whatever is simplest, since this is an edge case.

@Tug
Copy link
Contributor

Tug commented Mar 7, 2017

Code 👍
Not sure about the error message (what does it mean later for the user?).

@Tug
Copy link
Contributor

Tug commented Mar 7, 2017

Product 👍

@Tug Tug removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 7, 2017
@michelleweber
Copy link

Howdy! Editor at your disposal :)

Question: What does "bad" domain name mean here? Unavailable? Too short? Includes unacceptable characters? Something else? Is "This domain is unavailable" too vague here?

""X" isn't available as a domain. Keep going with setup -- you'll be able to choose another domain later."

@drewblaisdell
Copy link
Contributor

I propose we just go with whatever is simplest, since this is an edge case.

I don't think displaying a notice is the simplest solution here. I agree with @ranh that

Unless we know how users ended up with a bad domain query in the first place, the notice is not going to be very useful.

and that we should simply hard redirect the user to /domains once that route is available.

@ranh
Copy link
Contributor

ranh commented Mar 8, 2017

Thanks for the notes @michelleweber! I think we're going to remove this notice altogether though :) So clearing the copy review label.

@ranh ranh removed the [Status] Needs Copy Review Add this when you'd like to get a review / feedback from the Editorial team on your PR label Mar 8, 2017
@matticbot matticbot added [Size] S Small sized issue and removed [Size] M Medium sized issue labels Mar 8, 2017
@yurynix yurynix changed the title Display error message when the domain is invalid Domain only flow: redirect when starting the flow with a bad domain name Mar 8, 2017
@yurynix yurynix added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Author Reply labels Mar 8, 2017
@fabianapsimoes
Copy link
Contributor

Product 👍

Though I think we should test this again once we have the landing page at /domains. I'll add a note to #1425-tr.

@Tug
Copy link
Contributor

Tug commented Mar 8, 2017

Code 👍

@Tug Tug merged commit 8142264 into master Mar 8, 2017
@Tug Tug deleted the add/error-message-for-no-domain branch March 8, 2017 17:49
@matticbot matticbot removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] Ready to Merge labels Mar 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Checkout The checkout screen and process for purchases made on WordPress.com. [Size] S Small sized issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants